fix(#66): prevent inclusive tax double-counting on order totals and discounts#67
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes Issue #66
Root Cause: A systemic logic error existed in the subtotal and preRoundTotal accumulation loops where taxResult.tax_amount (or the proportionally scaled newTaxAmount during discount applications) was strictly appended to the base cost, irrespective of tax_type. Because products configured as tax_type = 'inclusive' inherently embed the true tax payload into their raw subtotal calculation (unit_price * quantity), the backend's uniform additive calculation step (total = subtotal + totalTax) caused an immediate double-counting bug. This structural duplication persisted during route mutations such as POST /api/orders, POST /:id/items, and dynamically scaled incorrectly via taxRatio multipliers in the PATCH discount routes.
Technical Implementation: We implement Option A as identified in the ticket by fundamentally segregating cumulative tax computations into discrete state variables across all mutating endpoints:
1.Accumulator Split: Injected exclusiveTax state bindings alongside existing totalTax loops within the codebase (main/routes/orders.ts). The system now conditionally forks accumulations during iteration (if (tax_type !== 'inclusive') exclusiveTax += item_tax).
2.preRoundTotal Recalibration: Fixed order creation (POST /api/orders) and addition pipelines (POST /:id/items) to strictly utilize exclusiveTax when deriving the preRoundTotal value. Inclusive items now cleanly pass through
itemTotal = itemSubtotal + 0 ensuring zero inflation footprint.
3.Proportional Discount Scaling: Overhauled the order-wide taxRatio / subtotal modifiers inside PATCH /:id/discount and PATCH /:id/items/:itemId/discount. The engine scales both buckets symmetrically for database retention (totalTax and inclusiveTax arrays remain 100% compliant and intact), but mathematically pipes only the derived newExclusiveTax node against the discountedSubtotal.
Validation:
1)Merged a new deterministic integration spec: tests/integration-inclusive-tax.test.ts
2)Explicitly asserts that single-additions bypass additive properties.
3)Asserts that dynamic percentage/flat-rate JSON discount reductions retain flat accounting integrity on inclusive tax matrices.
4)The standard, exclusive tax integration matrices uniformly continue yielding Exit Code 0.